Skip to content

Comments

feat(web): ui/ux cleanup#143

Open
skulldogged wants to merge 4 commits intospacedriveapp:mainfrom
skulldogged:ui-ux-improvements
Open

feat(web): ui/ux cleanup#143
skulldogged wants to merge 4 commits intospacedriveapp:mainfrom
skulldogged:ui-ux-improvements

Conversation

@skulldogged
Copy link
Contributor

Summary

Polishes the web UI across channels, skills, and overview pages, and fixes a few backend/API issues that were causing incorrect behavior in pagination and registry data.

Changes

  • Channels

    • Fixed branch/worker cards so collapsed text truncates correctly and expanded text wraps cleanly.
    • Improved branch/worker row alignment.
    • Fixed “load older messages” behavior (debounce + dedupe) and corrected backend timestamp pagination comparison.
  • Skills

    • Improved installed/browse card layout and alignment.
    • Updated installed-state action in browse cards (checkmark -> delete on hover/click).
    • Switched delete controls to neutral outline style.
    • Moved “Install from GitHub” section above the infinite list.
    • Added registry description support and surfaced total registry skill count.
  • Overview

    • Added/fixed edit links for routing, identity cards, memory, and configuration.
    • Improved activity heatmap/card sizing.
    • Updated memory donut card (title/center total) and fixed tooltip layering/tooltip sizing.
  • Shared UI

    • Standardized cortex chat toggle button styling across Channel, Memories, and Cortex pages (neutral style).
  • Backend

    • Fixed compile issues in browser tool URL parsing and API auth middleware extractor.
    • Added registry total passthrough and description enrichment fallback in skills API.

Testing

  • cargo check
  • npm run build

@bilawalriaz
Copy link
Contributor

I already have a PR for "Fixed compile issues in browser tool URL parsing and API auth middleware extractor" here

@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

Walkthrough

This PR extends public TypeScript interfaces with optional fields for skill descriptions and response totals, adds CSS class support to the Markdown component, refactors multiple agent detail/skills UIs with flex layouts and edit links, enhances channel detail rendering with Markdown whitespace preservation and load throttling, and implements async description enrichment for registry skills with updated SQL timestamp comparison.

Changes

Cohort / File(s) Summary
API Interface Extensions
interface/src/api/client.ts
Added optional description?: string field to RegistrySkill and total?: number field to RegistryBrowseResponse interfaces.
Component API Enhancement
interface/src/components/Markdown.tsx
Added optional className prop to Markdown component; applies provided class alongside base "markdown" class when rendering container div.
History Deduplication Logic
interface/src/hooks/useChannelLiveState.ts
Introduced itemKey() helper function for stable timeline item identification. Enhanced history merge logic to deduplicate already-seen items during loading and syncing by filtering out existing keys and prepending only new items; hasMore flag now derives from deduplicated results.
Agent Detail UI Restructuring
interface/src/routes/AgentDetail.tsx
Refactored memory, configuration, and model routing sections with flex layouts and per-item Edit links; replaced 7x24 grid approach for activity heatmap with responsive CSS grid layout; enhanced MemoryDonut with total value overlay; restructured IdentitySection to show per-item tabs with individual Edit links; adjusted component vertical alignment throughout.
Agent Skills Management UI & State Flow
interface/src/routes/AgentSkills.tsx
Reworked RegistrySkillCard and InstalledSkill UI with compact titles, adjusted spacing, and truncation. Introduced per-skill remove state flow with onRemove and isRemoving props. Replaced Set with Map for tracking installed skill names. Added GitHub install form under Browse tab. Implemented animated spinner and dynamic icon rendering for install/remove actions. Integrated total registry skill counter from browse response.
Channel Detail UI & Loading Optimization
interface/src/routes/ChannelDetail.tsx
Refactored branch/worker item header layouts with flex alignment and conditional truncation. Switched conclusions and results rendering to use Markdown component with whitespace preservation classes. Introduced lastLoadMoreAtRef timestamp-based throttling to debounce onLoadMore calls within 800ms window. Updated intersection observer logic with explicit entry intersection checks.
Backend Skills Enrichment Mechanism
src/api/skills.rs
Added description: Option<String> to RegistrySkill and total: Option<u64> to RegistryBrowseResponse. Implemented on-demand async description fetching from GitHub SKILL.md files with caching, markdown parsing, and text normalization during registry_browse and registry_search operations.
Data Layer Timestamp Handling
src/conversation/history.rs
Updated SQL filter in load_channel_timeline to use datetime() function wrapper for timestamp comparison, changing condition from timestamp < ?3 to datetime(timestamp) < datetime(?3) for improved timezone handling.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant API as skills.rs API
    participant Cache as Cache
    participant GitHub as GitHub
    
    Client->>API: registry_browse/search request
    activate API
    API->>Cache: check if descriptions cached for skills
    alt Descriptions found in cache
        Cache-->>API: cached descriptions
    else Descriptions not cached
        API->>GitHub: fetch SKILL.md from owner/repo
        activate GitHub
        GitHub-->>API: SKILL.md content
        deactivate GitHub
        API->>API: parse markdown & extract description
        API->>Cache: store description
    end
    API->>API: populate RegistrySkill.description & total
    API-->>Client: RegistryBrowseResponse with descriptions
    deactivate API
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Hopping through interfaces and APIs so grand,
Descriptions now enriched from distant GitHub land,
With flexing layouts, dedup magic, and edit links galore,
These UI and logic dances shall open every door,
Load times throttled smoothly, skills installed with flair!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat(web): ui/ux cleanup' is vague and generic, using non-descriptive terms that don't convey specific information about the substantial changes across multiple areas (channels, skills, overview, backend). Consider using a more specific title that highlights a primary change, such as 'feat(web): improve channel message loading and skills registry UI' or similar to better reflect the scope.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description comprehensively relates to the changeset, detailing specific improvements across channels, skills, overview, and backend with clear rationale for each area.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
interface/src/routes/ChannelDetail.tsx (1)

321-331: Minor indentation inconsistency.

The cortex toggle wrapper <div> at line 321 appears to break from the surrounding indentation level (it's inside the <div className="ml-auto flex items-center gap-3"> but doesn't share its indent). Not a functional issue, but it makes the JSX structure harder to read at a glance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@interface/src/routes/ChannelDetail.tsx` around lines 321 - 331, Indentation
of the cortex toggle wrapper div in ChannelDetail.tsx is inconsistent; adjust
the JSX so the <div className="flex overflow-hidden rounded-md border
border-app-line bg-app-darkBox"> (the cortex toggle wrapper containing the
Button that calls setCortexOpen and reads cortexOpen) is indented to match the
surrounding <div className="ml-auto flex items-center gap-3"> children, keeping
the Button and its props (onClick, variant, size, className, title) aligned with
the other sibling elements for consistent JSX formatting.
src/api/skills.rs (2)

256-263: Description enrichment adds latency to every browse/search response.

enrich_registry_descriptions runs inline in both registry_browse and registry_search handlers, blocking the response until all GitHub fetches complete (up to 3s timeout × 4 candidate paths per skill, though parallelized). First-time page loads with many uncached skills will be noticeably slow.

Consider enriching descriptions asynchronously (fire-and-forget background task that populates the cache) and returning whatever is cached, or returning partial results and letting the frontend re-fetch.

Also applies to: 306-313

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/skills.rs` around lines 256 - 263, The call to
enrich_registry_descriptions is blocking the HTTP response in registry_browse
and registry_search by awaiting GitHub fetches; change to fire-and-forget
background enrichment so the handlers return immediately with whatever
descriptions are already cached. Concretely, replace the await
enrich_registry_descriptions(&client, &mut skills).await with spawning a
background task (e.g., tokio::spawn or equivalent) that clones the needed
client/config and the list of skill identifiers and calls
enrich_registry_descriptions in the background to populate the cache, while the
handler returns Json(RegistryBrowseResponse { skills, ... }) (and do the same
for the RegistrySearchResponse branch). Ensure any shared cache or client is
cloned or Arc-wrapped and handle errors inside the task so the handler never
awaits it.

12-13: Unbounded in-memory cache will grow indefinitely.

REGISTRY_SKILL_DESCRIPTION_CACHE has no eviction policy, TTL, or size limit. Over time, as users browse/search different skills, this HashMap grows without bound. With thousands of registry skills, each storing a description string up to 220 chars, the memory impact is modest per entry but there's no ceiling.

Consider adding a bounded cache (e.g., an LRU with a max capacity) or a simple TTL-based eviction. Even a periodic clear on a timer would suffice.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/skills.rs` around lines 12 - 13, REGISTRY_SKILL_DESCRIPTION_CACHE is
an unbounded LazyLock<HashMap<String,String>> and will grow indefinitely;
replace it with a bounded, concurrent cache (e.g., an LRU or TTL cache) or add
periodic eviction: for example swap the LazyLock<RwLock<HashMap<...>>> for a
concurrent cache type (moka::sync::Cache or lru::LruCache behind a
tokio::sync::Mutex) with a configured max_capacity and/or TTL, update all
accesses that read/write REGISTRY_SKILL_DESCRIPTION_CACHE to use the cache's
get/insert/evict APIs (or implement a background task that periodically prunes
old entries if you prefer TTL), and ensure the chosen cache provides safe
async/concurrent usage in place of the current tokio::sync::RwLock wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@interface/src/routes/AgentSkills.tsx`:
- Around line 353-411: The GitHub form is sharing the global installMutation,
causing its UI states and messages to reflect installs triggered from registry
cards; create a separate mutation for the GitHub form (e.g.,
githubInstallMutation using the same useMutation handler currently used by
installMutation) and replace usages inside the form: onSubmit should call
githubInstallMutation.mutate(spec), the submit button disabled/isPending checks
should use githubInstallMutation.isPending, and the success/error messages
should use githubInstallMutation.isSuccess / githubInstallMutation.isError and
githubInstallMutation.data; leave the existing installMutation in place for
registry card buttons so their feedback remains scoped to registry installs.

In `@src/api/skills.rs`:
- Around line 393-394: The code currently builds raw URLs with a hardcoded
"main" branch (in the loop iterating candidate_paths and building url from
source and path), which fails for repos whose default branch is not main; change
the construction in that loop to resolve the default branch instead — either
call the GitHub REST API (GET /repos/{owner}/{repo}) to read default_branch and
use it when formatting url, or use the raw.githubusercontent.com "HEAD" redirect
by formatting url as "https://raw.githubusercontent.com/{source}/HEAD/{path}" so
the request follows the repo's default branch, and if you prefer robustness add
a small fallback sequence (try HEAD, then "main", then "master") when fetching
each candidate path.
- Around line 316-363: enrich_registry_descriptions is not caching "not found"
results so skills without SKILL.md trigger repeated HTTP calls; change the logic
to store a sentinel (e.g., an empty string or a dedicated marker) in
REGISTRY_SKILL_DESCRIPTION_CACHE when fetch_registry_skill_description returns
None, and when reading from REGISTRY_SKILL_DESCRIPTION_CACHE treat that sentinel
as meaning "looked up but no description" (i.e., set skills[index].description =
None/skip) so failed lookups aren't retried; update the join_set result handling
around fetch_registry_skill_description, the cache.insert call, and the initial
cached read to recognize and handle the sentinel (use registry_skill_key,
REGISTRY_SKILL_DESCRIPTION_CACHE, enrich_registry_descriptions, and
fetch_registry_skill_description to locate changes).

---

Nitpick comments:
In `@interface/src/routes/ChannelDetail.tsx`:
- Around line 321-331: Indentation of the cortex toggle wrapper div in
ChannelDetail.tsx is inconsistent; adjust the JSX so the <div className="flex
overflow-hidden rounded-md border border-app-line bg-app-darkBox"> (the cortex
toggle wrapper containing the Button that calls setCortexOpen and reads
cortexOpen) is indented to match the surrounding <div className="ml-auto flex
items-center gap-3"> children, keeping the Button and its props (onClick,
variant, size, className, title) aligned with the other sibling elements for
consistent JSX formatting.

In `@src/api/skills.rs`:
- Around line 256-263: The call to enrich_registry_descriptions is blocking the
HTTP response in registry_browse and registry_search by awaiting GitHub fetches;
change to fire-and-forget background enrichment so the handlers return
immediately with whatever descriptions are already cached. Concretely, replace
the await enrich_registry_descriptions(&client, &mut skills).await with spawning
a background task (e.g., tokio::spawn or equivalent) that clones the needed
client/config and the list of skill identifiers and calls
enrich_registry_descriptions in the background to populate the cache, while the
handler returns Json(RegistryBrowseResponse { skills, ... }) (and do the same
for the RegistrySearchResponse branch). Ensure any shared cache or client is
cloned or Arc-wrapped and handle errors inside the task so the handler never
awaits it.
- Around line 12-13: REGISTRY_SKILL_DESCRIPTION_CACHE is an unbounded
LazyLock<HashMap<String,String>> and will grow indefinitely; replace it with a
bounded, concurrent cache (e.g., an LRU or TTL cache) or add periodic eviction:
for example swap the LazyLock<RwLock<HashMap<...>>> for a concurrent cache type
(moka::sync::Cache or lru::LruCache behind a tokio::sync::Mutex) with a
configured max_capacity and/or TTL, update all accesses that read/write
REGISTRY_SKILL_DESCRIPTION_CACHE to use the cache's get/insert/evict APIs (or
implement a background task that periodically prunes old entries if you prefer
TTL), and ensure the chosen cache provides safe async/concurrent usage in place
of the current tokio::sync::RwLock wrapper.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7170ca0 and 66c9351.

📒 Files selected for processing (8)
  • interface/src/api/client.ts
  • interface/src/components/Markdown.tsx
  • interface/src/hooks/useChannelLiveState.ts
  • interface/src/routes/AgentDetail.tsx
  • interface/src/routes/AgentSkills.tsx
  • interface/src/routes/ChannelDetail.tsx
  • src/api/skills.rs
  • src/conversation/history.rs

Comment on lines +353 to +411
<div className="rounded-lg border border-app-line bg-app-box p-6">
<h3 className="text-sm font-medium text-ink">
Install from GitHub
</h3>
<p className="mt-1 text-xs text-ink-faint">
Install any skill from a GitHub repository
</p>
<form
onSubmit={(e) => {
e.preventDefault();
const formData = new FormData(e.currentTarget);
const spec = formData.get("spec") as string;
if (spec) {
installMutation.mutate(spec);
e.currentTarget.reset();
}
}}
className="mt-3 flex gap-2"
>
<input
type="text"
name="spec"
placeholder="owner/repo or owner/repo/skill-name"
className="flex-1 rounded-md border border-app-line bg-app-darkBox px-3 py-2 text-sm text-ink placeholder-ink-faint focus:border-accent focus:outline-none"
/>
<Button
type="submit"
variant="default"
size="default"
disabled={installMutation.isPending}
>
{installMutation.isPending ? (
<>
<FontAwesomeIcon
icon={faSpinner}
className="animate-spin"
/>
Installing...
</>
) : (
<>
<FontAwesomeIcon icon={faDownload} />
Install
</>
)}
</Button>
</form>
{installMutation.isError && (
<p className="mt-2 text-xs text-red-400">
Failed to install skill. Check the repository format.
</p>
)}
{installMutation.isSuccess && (
<p className="mt-2 text-xs text-green-400">
Installed {installMutation.data.installed.length} skill(s):{" "}
{installMutation.data.installed.join(", ")}
</p>
)}
</div>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Shared installMutation causes leaked feedback between the GitHub form and registry cards.

The "Install from GitHub" form and the registry card install buttons share the same installMutation. This means:

  • The form's success message (lines 405–410) will display even when a registry card triggered the install.
  • The form's "Installing..." state (line 382) activates when any registry card install is pending.
  • installMutation.isError under the form fires for registry card failures too.

Consider using a separate mutation for the GitHub form, or at minimum, track which source triggered the install to scope the feedback messages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@interface/src/routes/AgentSkills.tsx` around lines 353 - 411, The GitHub form
is sharing the global installMutation, causing its UI states and messages to
reflect installs triggered from registry cards; create a separate mutation for
the GitHub form (e.g., githubInstallMutation using the same useMutation handler
currently used by installMutation) and replace usages inside the form: onSubmit
should call githubInstallMutation.mutate(spec), the submit button
disabled/isPending checks should use githubInstallMutation.isPending, and the
success/error messages should use githubInstallMutation.isSuccess /
githubInstallMutation.isError and githubInstallMutation.data; leave the existing
installMutation in place for registry card buttons so their feedback remains
scoped to registry installs.

Comment on lines +316 to +363
async fn enrich_registry_descriptions(client: &reqwest::Client, skills: &mut [RegistrySkill]) {
let mut join_set = tokio::task::JoinSet::new();

for index in 0..skills.len() {
if skills[index]
.description
.as_ref()
.is_some_and(|description| !description.trim().is_empty())
{
continue;
}

let source = skills[index].source.clone();
let skill_id = skills[index].skill_id.clone();
let cache_key = registry_skill_key(&source, &skill_id);

let cached = {
let cache = REGISTRY_SKILL_DESCRIPTION_CACHE.read().await;
cache.get(&cache_key).cloned()
};

if let Some(description) = cached {
skills[index].description = Some(description);
continue;
}

let client = client.clone();
join_set.spawn(async move {
let description = fetch_registry_skill_description(&client, &source, &skill_id).await;
(index, cache_key, description)
});
}

while let Some(result) = join_set.join_next().await {
let Ok((index, cache_key, description)) = result else {
continue;
};
let Some(description) = description else {
continue;
};

if let Some(skill) = skills.get_mut(index) {
skill.description = Some(description.clone());
}

let mut cache = REGISTRY_SKILL_DESCRIPTION_CACHE.write().await;
cache.insert(cache_key, description);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

No negative caching causes repeated HTTP storms for skills without SKILL.md.

When fetch_registry_skill_description returns None (lines 353-355), nothing is cached. On the next browse/search request, the same skill triggers another round of up to 4 HTTP requests to GitHub. For skills that genuinely have no SKILL.md, this repeats on every page load.

Cache a sentinel value (e.g., empty string or a dedicated "not found" marker) so failed lookups aren't retried every time.

Proposed fix
     while let Some(result) = join_set.join_next().await {
         let Ok((index, cache_key, description)) = result else {
             continue;
         };
-        let Some(description) = description else {
-            continue;
-        };
+        let description = description.unwrap_or_default();
 
         if let Some(skill) = skills.get_mut(index) {
-            skill.description = Some(description.clone());
+            if !description.is_empty() {
+                skill.description = Some(description.clone());
+            }
         }
 
         let mut cache = REGISTRY_SKILL_DESCRIPTION_CACHE.write().await;
         cache.insert(cache_key, description);
     }

Then when reading from the cache, treat an empty string as "already looked up, no description found":

         if let Some(description) = cached {
-            skills[index].description = Some(description);
+            if !description.is_empty() {
+                skills[index].description = Some(description);
+            }
             continue;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async fn enrich_registry_descriptions(client: &reqwest::Client, skills: &mut [RegistrySkill]) {
let mut join_set = tokio::task::JoinSet::new();
for index in 0..skills.len() {
if skills[index]
.description
.as_ref()
.is_some_and(|description| !description.trim().is_empty())
{
continue;
}
let source = skills[index].source.clone();
let skill_id = skills[index].skill_id.clone();
let cache_key = registry_skill_key(&source, &skill_id);
let cached = {
let cache = REGISTRY_SKILL_DESCRIPTION_CACHE.read().await;
cache.get(&cache_key).cloned()
};
if let Some(description) = cached {
skills[index].description = Some(description);
continue;
}
let client = client.clone();
join_set.spawn(async move {
let description = fetch_registry_skill_description(&client, &source, &skill_id).await;
(index, cache_key, description)
});
}
while let Some(result) = join_set.join_next().await {
let Ok((index, cache_key, description)) = result else {
continue;
};
let Some(description) = description else {
continue;
};
if let Some(skill) = skills.get_mut(index) {
skill.description = Some(description.clone());
}
let mut cache = REGISTRY_SKILL_DESCRIPTION_CACHE.write().await;
cache.insert(cache_key, description);
}
async fn enrich_registry_descriptions(client: &reqwest::Client, skills: &mut [RegistrySkill]) {
let mut join_set = tokio::task::JoinSet::new();
for index in 0..skills.len() {
if skills[index]
.description
.as_ref()
.is_some_and(|description| !description.trim().is_empty())
{
continue;
}
let source = skills[index].source.clone();
let skill_id = skills[index].skill_id.clone();
let cache_key = registry_skill_key(&source, &skill_id);
let cached = {
let cache = REGISTRY_SKILL_DESCRIPTION_CACHE.read().await;
cache.get(&cache_key).cloned()
};
if let Some(description) = cached {
if !description.is_empty() {
skills[index].description = Some(description);
}
continue;
}
let client = client.clone();
join_set.spawn(async move {
let description = fetch_registry_skill_description(&client, &source, &skill_id).await;
(index, cache_key, description)
});
}
while let Some(result) = join_set.join_next().await {
let Ok((index, cache_key, description)) = result else {
continue;
};
let description = description.unwrap_or_default();
if let Some(skill) = skills.get_mut(index) {
if !description.is_empty() {
skill.description = Some(description.clone());
}
}
let mut cache = REGISTRY_SKILL_DESCRIPTION_CACHE.write().await;
cache.insert(cache_key, description);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/skills.rs` around lines 316 - 363, enrich_registry_descriptions is
not caching "not found" results so skills without SKILL.md trigger repeated HTTP
calls; change the logic to store a sentinel (e.g., an empty string or a
dedicated marker) in REGISTRY_SKILL_DESCRIPTION_CACHE when
fetch_registry_skill_description returns None, and when reading from
REGISTRY_SKILL_DESCRIPTION_CACHE treat that sentinel as meaning "looked up but
no description" (i.e., set skills[index].description = None/skip) so failed
lookups aren't retried; update the join_set result handling around
fetch_registry_skill_description, the cache.insert call, and the initial cached
read to recognize and handle the sentinel (use registry_skill_key,
REGISTRY_SKILL_DESCRIPTION_CACHE, enrich_registry_descriptions, and
fetch_registry_skill_description to locate changes).

Comment on lines +393 to +394
for path in candidate_paths.drain(..) {
let url = format!("https://raw.githubusercontent.com/{source}/main/{path}");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded main branch will miss repos using master or other default branches.

Line 394 always constructs URLs with /main/. Repos that use master, develop, or any other default branch name will never resolve.

Consider trying a fallback branch (e.g., master) or using the GitHub API to resolve the default branch. Alternatively, GitHub's raw content CDN redirects HEAD → default branch if you use the API endpoint instead of raw.githubusercontent.com.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/skills.rs` around lines 393 - 394, The code currently builds raw URLs
with a hardcoded "main" branch (in the loop iterating candidate_paths and
building url from source and path), which fails for repos whose default branch
is not main; change the construction in that loop to resolve the default branch
instead — either call the GitHub REST API (GET /repos/{owner}/{repo}) to read
default_branch and use it when formatting url, or use the
raw.githubusercontent.com "HEAD" redirect by formatting url as
"https://raw.githubusercontent.com/{source}/HEAD/{path}" so the request follows
the repo's default branch, and if you prefer robustness add a small fallback
sequence (try HEAD, then "main", then "master") when fetching each candidate
path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants